Skip to content

fix: hold terminal input until a fresh pane's agent TUI is ready - #3105

Open
ikeshavvarshney wants to merge 5 commits into
Untrivial-ai:mainfrom
ikeshavvarshney:fix/terminal-input-before-agent-ready-3023
Open

fix: hold terminal input until a fresh pane's agent TUI is ready#3105
ikeshavvarshney wants to merge 5 commits into
Untrivial-ai:mainfrom
ikeshavvarshney:fix/terminal-input-before-agent-ready-3023

Conversation

@ikeshavvarshney

Copy link
Copy Markdown
Contributor

Summary

Fixes #3023 — terminal accepted keystrokes as soon as tmux attach succeeded, before the launched agent's TUI had entered raw mode. Fast typing right after opening a fresh session landed as raw, echoed text in the pane's scrollback instead of the agent's input box, and was unrecoverable there.

Root cause

Attach-readiness and agent-readiness are two different things. tmux forwards bytes to the pane's foreground process the moment attach succeeds — regardless of whether that process has actually read stdin yet. A full-screen TUI agent (Claude Code, Codex, ...) can take a moment after launch to enter raw mode, and until it does, the underlying line discipline echoes typed characters straight into the terminal's canonical-mode output.

Fix

Hold input on a pane's first-ever attach for a short grace window (500ms), reusing the existing pendingInput buffering in attachment.write() — no new buffering logic needed. The window is tracked per pane id in Manager (inputHoldDeadline), keyed off the first time this daemon process sees that id, so:

  • A pane's genuine first attach (freshly launched agent) waits.
  • Any later reattach, tab reopen, or second client viewing the same pane reuses the already-ticking (or already-expired) deadline — never re-delays.
  • Fully configurable/disableable via WithInputGraceWindow (defaults to 500ms, tests disable it for determinism).
  • Output streaming is untouched — only accepting input is delayed, so the user still sees the agent boot in real time.

Changes

  • backend/internal/terminal/manager.goinputHoldDeadline first-seen tracking per pane id, WithInputGraceWindow option, default 500ms grace window.
  • backend/internal/terminal/attachment.gosetPTY waits out the hold (if any) on a pane's first attach before flipping inputReady, cancellable via ctx so shutdown/close is never blocked.
  • backend/internal/terminal/manager_test.go — two new tests (TestServeFreshPaneHoldsInputUntilGraceWindowElapses, TestServeReattachSkipsInputGraceWindow) plus WithInputGraceWindow(0) added to existing timing-sensitive tests to keep them deterministic.

Test plan

  • go build ./...
  • go test ./internal/terminal/... — full suite (33 tests) passes, including the two new grace-window tests.
  • gofmt clean.
  • Manually verified in the real Electron app: spawned a fresh Claude Code session, typed immediately on terminal attach — text lands cleanly in Claude Code's input line once its TUI paints, boot banner above stays untouched (previously the text would have scattered into the banner/scrollback).

06-settled-more ---

tmux forwards keystrokes to a pane's foreground process the instant
attach succeeds, whether or not that process has entered raw mode yet.
A full-screen TUI agent (Claude Code, Codex, ...) can take a moment
after launch to do so, so fast typing right after a session opens
lands as raw, echoed text in the scrollback instead of the agent's
input box.

Hold input on a pane's first-ever attach for a short grace window,
buffering keystrokes via the existing pendingInput mechanism instead
of forwarding them early. Only a pane's genuine first attach waits;
reattaching to an already-running pane or a second client is
unaffected.

Fixes Untrivial-ai#3023
@somewherelostt

Copy link
Copy Markdown
Collaborator

Thanks for contributing to Agent Orchestrator.

This PR is being picked up by the current external contributor on-call pair:

If someone is already working on this, please continue as usual.
The on-call pair is added for visibility, tracking, and support, not to take over the work.
If you need help with review, direction, reproduction, or next steps, please tag @illegalcall and @Pulkit7070 here.

For faster context or live questions, you can also join the AO Discord.

Join the session here:
https://discord.gg/H6ZDcUXmq

Come by if you want to see what is being built, ask questions, or just hang around with the community.

Comment thread backend/internal/terminal/manager.go Outdated
Comment thread backend/internal/terminal/attachment.go Outdated
Comment thread backend/internal/terminal/manager.go Outdated
Addresses review on the terminal input-hold fix (AgentWrapper#3023):

- The hold now starts on a pane's first SUCCESSFUL PTY publish instead
  of at open-request time. A shared one-shot inputGate per pane id
  replaces the eager deadline: only setPTY's own successful attach can
  arm it, so a slow Attach or a failed-then-retried attach can no
  longer let the window expire before the pane is even up.
- setPTY now publishes the Stream and returns immediately so copyOut
  starts reading output right away; releasing buffered input happens
  on a separate goroutine gated on the shared inputGate, so a boot-time
  output burst is no longer held hostage behind the input hold. The
  release goroutine checks the Stream is still current before flushing,
  so a stale waiter can't write into a Stream a later reattach replaced.
- The gate is kept for the Manager's lifetime instead of being
  time-pruned, so reopening a long-running pane can never be mistaken
  for a fresh one and re-delayed.

Adds regression tests for a slow attach, a failed-then-successful
attach, output streaming during the hold, and a reattach long after the
window elapsed.
@ikeshavvarshney

Copy link
Copy Markdown
Contributor Author

Fixed all three. Gate redesigned per your suggestion: shared one-shot inputGate per pane id, armed only by setPTY's own first successful publish — so a slow Attach or a failed-then-retried attach can no longer let the window expire early or start prematurely. setPTY now returns immediately after publishing so copyOut streams output right away; input release moved to a background goroutine gated on inputGate, checking a.pty == p before flushing so a stale waiter can't write into a superseded Stream. Gate is kept for the Manager's lifetime, no more time-pruning, so a long-running reattach never gets re-delayed.

Added the four regression tests requested: slow-attach (window measured from attach success, not open request), failed-then-successful attach, output-not-stalled-during-hold, and no-new-delay-on-late-reattach. Full suite (38 tests) passes, gofmt clean, and re-verified live in a real Claude Code session — no leaked/garbled boot text, output streamed immediately.

Comment thread backend/internal/terminal/manager.go
Comment thread backend/internal/terminal/attachment.go Outdated
Comment thread backend/internal/terminal/manager.go Outdated
Comment thread backend/internal/terminal/attachment.go Outdated
Comment thread backend/internal/terminal/attachment.go Outdated
…tion-safe

Addresses a second review round on the terminal input-hold fix (Untrivial-ai#3023):

- Reset the pane's shared input gate on every genuine (re)launch of its
  agent process (terminal.Manager.ResetInputGate), wired through a new
  session_manager.InputGateResetter hook called from relaunchSession.
  Without this, a resume/restart that reuses the runtime handle
  (ports.RuntimeRestarter) would inherit the exited process's
  already-open gate and let the replacement TUI's early keystrokes
  through unheld, reproducing the original race on every resume.
- releaseInput now re-reads the live Stream fresh before every single
  buffered write instead of holding one Stream reference for the whole
  drain, guarded by a draining flag so only one drain loop runs per
  attachment at a time. A Stream swap mid-drain (a reattach racing the
  release) now hands the remainder to whichever Stream is current,
  instead of risking a write into one this attachment no longer owns.
- inputGate.wait now reports whether the gate actually opened or ctx
  ended first; releaseInputWhenGateOpens abandons on the latter. Manager
  ../Close cancels its context before marking attachments closed, so a
  waiter could previously flush buffered keystrokes mid-shutdown.
- A genuine (still-current) write failure during the async flush now
  cancels the attach loop and closes the Stream, so run's blocked
  copyOut unblocks and the loop actually exits instead of treating a
  dead Stream as an ordinary drop-and-reattach case.
- releaseInput takes a context.Context (AGENTS.md: ctx first-arg for
  I/O/blocking work) and checks cancellation before draining each chunk.

Adds regression tests for: a resume resetting the gate, a Stream swap
mid-drain preserving order across the two Streams, Manager.Close
abandoning a mid-hold flush promptly, and an async flush failure
closing its Stream and ending the attach loop.
@ikeshavvarshney

Copy link
Copy Markdown
Contributor Author

Fixed all five.

  • Generation-safe input gateterminal.Manager.ResetInputGate is now reset on every relaunch (resume/restart included) via a new InputGateResetter hook wired from relaunchSession. A reused runtime handle no longer inherits the exited process's already-open gate.

  • Safe drain on reattach — drain now re-reads the live Stream before every chunk write, guarded by a draining flag, so a reattach mid-flush hands the remainder to the new Stream instead of risking a misdelivered write.

  • Explicit gate outcomeinputGate.wait now reports open-vs-cancelled explicitly. Close-during-hold abandons cleanly.

  • Attach loop exits correctly on write failure — a genuine write failure now cancels the attach loop and closes the Stream so run() actually exits instead of hanging in copyOut.

  • Context propagationreleaseInput now takes ctx per AGENTS.md convention.

Added the four requested regressions:

  • Resume resets input gate.
  • Stream-swap ordering across two buffered writes.
  • Close-during-hold writes zero bytes.
  • Async flush failure closes the stream and ends run().

Also added the session manager wiring test.

Full suite green, gofmt clean, and re-verified live in the real Electron app against a fresh Claude Code session.

The GitHub web UI conflict resolution for the sessionLifecycle merge
(InputGateResetter + ShellTerminalCloser) concatenated the two method
bodies without a separating blank line, which gofmt/goimports requires
between a func and the next top-level declaration. Fixes the golangci-lint
goimports CI failure on backend/internal/session_manager/manager.go:290.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: terminal accepts input before agent UI is ready, causing first keystrokes to render outside input box

5 participants